Enable health check to use mtls to connect to backends#807
Closed
felicity3786 wants to merge 3 commits into
Closed
Enable health check to use mtls to connect to backends#807felicity3786 wants to merge 3 commits into
felicity3786 wants to merge 3 commits into
Conversation
oneonestar
reviewed
Dec 12, 2025
Comment on lines
+145
to
+146
| MONITOR_HTTP_CLIENT_TRUST_STORE_PATH, | ||
| MONITOR_HTTP_CLIENT_TRUST_STORE_PASSWORD |
Member
There was a problem hiding this comment.
A user could skip setting truststore. Airlift would fallback to some system default truststore.
| } | ||
| else { | ||
| identityHeader = new Header("X-Trino-User", backendStateConfiguration.getUsername()); | ||
| identityHeader = null; |
Member
There was a problem hiding this comment.
Please use Optional instead of null.
Peiyingy
reviewed
Dec 12, 2025
Comment on lines
+85
to
+89
| private static final String MONITOR_HTTP_CLIENT_KEY_STORE_PATH = "monitor.http-client.key-store-path"; | ||
| private static final String MONITOR_HTTP_CLIENT_KEY_STORE_PASSWORD = "monitor.http-client.key-store-password"; | ||
| private static final String MONITOR_HTTP_CLIENT_TRUST_STORE_PATH = "monitor.http-client.trust-store-path"; | ||
| private static final String MONITOR_HTTP_CLIENT_TRUST_STORE_PASSWORD = "monitor.http-client.trust-store-password"; | ||
|
|
Member
There was a problem hiding this comment.
Actually I was thinking of something like this, WDYT?
Suggested change
| private static final String MONITOR_HTTP_CLIENT_KEY_STORE_PATH = "monitor.http-client.key-store-path"; | |
| private static final String MONITOR_HTTP_CLIENT_KEY_STORE_PASSWORD = "monitor.http-client.key-store-password"; | |
| private static final String MONITOR_HTTP_CLIENT_TRUST_STORE_PATH = "monitor.http-client.trust-store-path"; | |
| private static final String MONITOR_HTTP_CLIENT_TRUST_STORE_PASSWORD = "monitor.http-client.trust-store-password"; | |
| private static final List<String> REQUIRED_MONITOR_MTLS_KEYS = List.of( | |
| "monitor.http-client.key-store-path", | |
| "monitor.http-client.key-store-password", | |
| "monitor.http-client.trust-store-path", | |
| "monitor.http-client.trust-store-password"); |
|
This pull request has gone a while without any activity. Ask for help on #trino-gateway-dev on Trino slack. |
|
Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time. |
Member
|
I think this PR still provides a useful feature. What do you think @felicity3786 @oneonestar .. if yes we should reopen and continue work on it ideally. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Today, Trino Gateway’s health monitors authenticate to backend clusters using either:
This creates operational burdens for deployments that must rotate or manage shared credentials, and it exposes unnecessary authentication material in gateway configs.
This PR introduces optional mTLS-based authentication for health checks, allowing deployments to rely solely on client certificates when connecting to backend JMX or metrics endpoints.
backendState.monitorMtlsEnabled: true/falseWhen
monitorMtlsEnabled=true, the health monitors switch to certificate-based auth and no longer send any identity headers. Startup validation for TLS required configuration is also added.If
monitorMtlsEnabledis false (default), existing behavior is unchanged, provides the backward compatibility.Test
Build passed.
Tested with local gateway the health check is working as expected.
Additional context and related issues
NA
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required, with the following suggested text:
* Fix some things.